feat: support using directive for scala-cli #237

tanishiking · 2022-07-20T16:46:47Z

This PR adds limited support for the using directive of scala-cli.
Even though the using directives aren't currently a part of the Scala language standard (while there's a PRE-SIP for it, we'd like to have proper syntax highlighting for it.

Based on this change, I'd like to hear some opinions from compiler team :)

Is this

acceptable, or
should we wait until the using-directive syntax //> (decided to) becomes a part of scala standard or
should we wait until scala-cli becomes mature enough to be a standard tool for Scala?

While scala-cli allows us to write various forms of using directives, we focus on one-line, comment-based using directives such as //> using scala 3.1.3 as it's the most recommended syntax by scala-cli.

For now we recommend using the special comment (//> using scala "3.0.2"), and we will use that syntax in this guide.
https://scala-cli.virtuslab.org/docs/guides/using-directives

What we don't support at this moment

using directives without `//>`

scala-cli accepts something like

using scala 2.13

def main = ...

However, according to the discussion in the PRE-SIP, we're leaning towards not introducing the using directives without special comment, //>.

After many discussions, I get convinced to stick to the comment syntax for now. Usability gains with dropping special syntax for comments are small, and there is non-zero potential to confuse users. It also seems that early adopters of Scala CLI are content with comment-based syntax.
https://contributors.scala-lang.org/t/pre-sip-using-directives/5700/16

Therefore, we don't support it in this PR.

multiline using directive

Also, I realized that multiline using directive is also valid

  //> using
  //>   oneKey ...
  //>   secondKey ...
  //>   thirdKey {
  //>     nestedKey1
  //>     nestedKey2
  //>   }

but I would leave it out of scope for now, because it's difficult to capture (even I'm not sure it's possible) with the regex-based TextMate grammar AFAIK. (it would be much easier if we have tokens that represent the start and end of the directive).

/>using /

We can support it but I just didn't for now, because I'd like to hear people's opinions before going further.

FYI @Gedochao @romanowski @tgodzik Also, it would be good to play around with this build and see how it colorizes the using directives. See the CONTRIBUTING guide for how to build locally https://github.com/scala/vscode-scala-syntax/blob/main/CONTRIBUTING.md

Here's the example

tgodzik · 2022-07-21T10:01:35Z

Looks great! What do you think @nicolasstucki ? Would it be ok to include this for Scala syntax?

Gedochao · 2022-07-21T12:07:33Z

Hey, awesome stuff @tanishiking!
I played with it a bit and I already love what's there, but I feel it might still need a lil' bit of work.

Screen.Recording.2022-07-21.at.13.55.23.mov

Although complete and correct directives are colored as they should, as far as I tested, incomplete ones seem to mess with the coloring of the following lines with regular Scala code (refer to the short clip attached).

As in, you may notice that an open quote for the lib dependency string colors the following lines (even though they aren't commented and shouldn't be considered a part of a directive, at any time). Similarly, if the code for the object Hello start a line below the incomplete //> using lib, the bottom bracket becomes red for some reason.

Still, hyped for the end result, this is promising!

related to: VirtusLab/scala-cli#1059

tanishiking · 2022-07-22T08:22:52Z

@Gedochao Hey, thank you for taking a look!

if the code for the object Hello start a line below the incomplete //> using lib, the bottom bracket becomes red for some reason.

Fixed by changing the begin of #using-directive not to capture newline as a part of begin. Previously,
//> using\n was accepted as a begin of using-directive and the following lines (object Hello in your example) as colored as if they are parts of using-directive. Now it's fixed :)

- begin: '^\\s*(//>)\\s*(using)\\n+'
+ begin: '^\\s*(//>)\\s*(using)[^\\S\\n]+'

an open quote for the lib dependency string colors the following lines (even though they aren't commented and shouldn't be considered a part of a directive, at any time)

I believe it should be a correct behavior because

1. "" inside of using-directives should be colored the same as the "" in the program. This behavior (incomplete " make the following code as if they're string) (even though multiline string with "..." is invalid).
1. I think this wired coloring for incomplete " provides a good experience to users:
- users can easily notice something is wrong

prolativ · 2022-07-22T09:16:34Z

Something to keep in mind for the future: if scala-cli gets support for path interpolations VirtusLab/scala-cli#1098 they also should get coloured properly

Gedochao · 2022-07-25T08:23:11Z

Fixed by changing the begin of #using-directive not to capture newline as a part of begin. Previously, //> using\n was accepted as a begin of using-directive and the following lines (object Hello in your example) as colored as if they are parts of using-directive. Now it's fixed :)
- begin: '^\\s*(//>)\\s*(using)\\n+'
+ begin: '^\\s*(//>)\\s*(using)[^\\S\\n]+'

Just tested, works as a charm 🚀

an open quote for the lib dependency string colors the following lines (even though they aren't commented and shouldn't be considered a part of a directive, at any time)

I believe it should be a correct behavior because

"" inside of using-directives should be colored the same as the "" in the program. This behavior (incomplete " make the following code as if they're string) (even though multiline string with "..." is invalid).

I think this wired coloring for incomplete " provides a good experience to users:

users can easily notice something is wrong

@tanishiking I see your points, however, for as long as using directives are contained in comments (with the comment syntax), I do believe it to be confusing that something within a comment could influence anything at all outside.

So an open quote for a string like on this screenshot goes against that, coloring stuff that's uncommented.
I do agree it would make sense to color the following lines to signal to the user that he left the string open, but IMO it should only color the subsequent lines that actually start with //>.

A third opinion may be necessary on this.

tanishiking · 2022-07-25T08:38:13Z

From a technical point of view, if we don't wanna color incomplete double quotation in //> we can't re-use include { "#strings" }

vscode-scala-syntax/src/typescript/Scala.tmLanguage.ts

Line 312 in fbd1508

strings: {

and need to define special coloring rule for a string in using directive such as

Work as a normal string if it's complete
Do not color it if there are no closing quotations in a line
- I'm not sure how we can make it 🤔

Gedochao · 2022-07-25T08:49:59Z

No idea how to approach it for now, but just one more thought - what's in this PR is already a huge improvement, with syntax coloring for using directive basically already working fine.

If fixing unclosed quotation within a directive turns out to be a more complex thing, I think it might make sense to extract it to a separate issue and address it separately in the future, while for the time being we push what's already here.

feat: support using directive for scala-cli

31ac9b0

related to: VirtusLab/scala-cli#1059

tanishiking force-pushed the using-directive branch from 62016bf to 31ac9b0 Compare July 22, 2022 08:05

tgodzik approved these changes Jul 26, 2022

View reviewed changes

tgodzik merged commit a719fc9 into scala:main Jul 26, 2022

tanishiking deleted the using-directive branch July 27, 2022 08:47

lwronski mentioned this pull request Sep 13, 2022

Syntax coloring for using directives in Metals VirtusLab/scala-cli#1059

Closed

tgodzik mentioned this pull request Nov 14, 2022

Support syntax highlight for multiline using directives VirtusLab/scala-cli#1578

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support using directive for scala-cli #237

feat: support using directive for scala-cli #237

tanishiking commented Jul 20, 2022 •

edited

Loading

tgodzik commented Jul 21, 2022

Gedochao commented Jul 21, 2022

tanishiking commented Jul 22, 2022

prolativ commented Jul 22, 2022

Gedochao commented Jul 25, 2022

tanishiking commented Jul 25, 2022

Gedochao commented Jul 25, 2022

feat: support using directive for scala-cli #237

feat: support using directive for scala-cli #237

Conversation

tanishiking commented Jul 20, 2022 • edited Loading

What we don't support at this moment

using directives without //>

multiline using directive

/*>using */

tgodzik commented Jul 21, 2022

Gedochao commented Jul 21, 2022

tanishiking commented Jul 22, 2022

prolativ commented Jul 22, 2022

Gedochao commented Jul 25, 2022

tanishiking commented Jul 25, 2022

Gedochao commented Jul 25, 2022

tanishiking commented Jul 20, 2022 •

edited

Loading

using directives without `//>`

/>using /